Unfortunately, the assertion failure is not easy to reproduce in a test because it doesn't happen all the time. It typically requires a clean-slate session. Might spend some more time doing a test for it later.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.32%. Comparing base (59f50f2) to head (f390b0c).
Report is 744 commits behind head on main.

@@            Coverage Diff             @@
##             main    #7515      +/-   ##
==========================================
+ Coverage   80.06%   81.32%   +1.25%     
==========================================
  Files         190      242      +52     
  Lines       37181    44891    +7710     
  Branches     9450    11204    +1754     
==========================================
+ Hits        29770    36507    +6737     
- Misses       2997     3990     +993     
+ Partials     4414     4394      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Here's a script that reproduces the assertion crash:

select setseed(0.2);

create table readings(time timestamptz, device int, temp float);

select create_hypertable('readings', 'time', create_default_indexes => false);

insert into readings
select t, ceil(random()*10), random()*40
from generate_series('2022-06-01'::timestamptz, '2022-06-20 00:01:00', '1s') t;

alter table readings set (
      timescaledb.compress,
      timescaledb.compress_orderby = 'time',
      timescaledb.compress_segmentby = 'device'
);

--insert into readings values ('2022-06-01', 1, 1.0), ('2022-06-02', 2, 2.0), ('2022-08-02', 3, 3.0);

create index on readings (time);

select format('%I.%I', chunk_schema, chunk_name)::regclass as chunk
  from timescaledb_information.chunks
 where format('%I.%I', hypertable_schema, hypertable_name)::regclass = 'readings'::regclass
 limit 1 \gset

set timescaledb.enable_chunk_append=off;

select compress_chunk(ch) from show_chunks('readings') ch;
select decompress_chunk('_timescaledb_internal._hyper_1_3_chunk');

--set timescaledb.enable_chunk_append=on;

explain (analyze)
select time, avg(temp), device  from readings
where time  > now() - interval '2 years 5 months 20 days' group by time, device order by time;

@akuzm After some more investigation it seems this is related to parallel query plans, where the spawned workers don't have the relation locked. I don't know if parallel workers are expected to have relations referenced in plan nodes locked at the beginning of plan execution.

@akuzm After some more investigation it seems this is related to parallel query plans, where the spawned workers don't have the relation locked. I don't know if parallel workers are expected to have relations referenced in plan nodes locked at the beginning of plan execution.

Not sure, e.g. the parallel seq scans seemingly doesn't lock tables in the workers, athough I only looked through the code and didn't check. In ChunkAppend, we ultimately switched to performing chunk exclusion in the parallel leader. Maybe it makes sense to do the same here as well. #5857

@akuzm After some more investigation it seems this is related to parallel query plans, where the spawned workers don't have the relation locked. I don't know if parallel workers are expected to have relations referenced in plan nodes locked at the beginning of plan execution.

Not sure, e.g. the parallel seq scans seemingly doesn't lock tables in the workers, athough I only looked through the code and didn't check. In ChunkAppend, we ultimately switched to performing chunk exclusion in the parallel leader. Maybe it makes sense to do the same here as well. #5857

If we can do chunk exclusion only once it is better. But TBH, I am not sure it is worth the effort given how seldom this scan node is used, so for now I think just taking this lock in the worker is enough. We can always implement leader exclusion later.

@akuzm Are we good to proceed with this fix. It seems the lack of locking is only in parallel queries so it seems correct to grab the lock if it is not already taken. Also seems like a very low-risk fix.

@akuzm Are we good to proceed with this fix. It seems the lack of locking is only in parallel queries so it seems correct to grab the lock if it is not already taken. Also seems like a very low-risk fix.

Sure. Let's move the lock before constify_restrictinfos, that's a complicated function that is usually called during planning, it might have some obscure path that requires a lock as well.

@akuzm Are we good to proceed with this fix. It seems the lack of locking is only in parallel queries so it seems correct to grab the lock if it is not already taken. Also seems like a very low-risk fix.

Sure. Let's move the lock before constify_restrictinfos, that's a complicated function that is usually called during planning, it might have some obscure path that requires a lock as well.

Moved the lock to before constify_restrictinfos.